Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: relay style pagination not supporting non-root args #8524 #10746

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: relay style pagination not supporting non-root args #8524 #10746

wants to merge 1 commit into from

Conversation

zanechua
Copy link

@zanechua zanechua commented Apr 8, 2023

Checklist:

  • If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

@netlify
Copy link

netlify bot commented Apr 8, 2023 •
edited
Loading

‼️ Deploy request for apollo-client-docs rejected.

Name Link
🔨 Latest commit cb28c87

@changeset-bot
Copy link

changeset-bot bot commented Apr 8, 2023

⚠️ No Changeset found

Latest commit: cb28c87

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@alessbell
Copy link
Contributor

Hi @zanechua 👋

Thanks for the PR - would you be able to add some tests here to illustrate problem and fix? If you don't have the bandwidth right now that's not a problem; in that case it would be helpful if you could link to the issue—or open one if one doesn't yet exist—so we can help move this along. Thanks again for this contribution!

@alessbell alessbell added the 🏓 awaiting-contributor-response requires input from a contributor label Apr 10, 2023
@zanechua
Copy link
Author

zanechua commented Apr 13, 2023 •
edited
Loading

Hey @alessbell

This MR actually fixes #8524

Was mentioned in the commit but I think it might not have been obvious.

Let me see if I can write the tests for it.

@jerelmiller jerelmiller added 🏓 awaiting-team-response requires input from the apollo team 👩‍🔬 needs-more-tests and removed 🏓 awaiting-contributor-response requires input from a contributor 🏓 awaiting-team-response requires input from the apollo team labels Jul 10, 2023
@jerelmiller
Copy link
Member

Hey @zanechua 👋

I wanted to give a gentle nudge here and see if you're still interested in moving this work forward. No worries if not! I know sometimes things can get lost in the shuffle. Thanks!

@zanechua
Copy link
Author

Hey @jerelmiller thanks for the ping.

Do you know where I can find the existing test files for this?

@phryneas
Copy link
Member

@zanechua you can find the current tests for relayStylePagination at https://github.com/apollographql/apollo-client/blob/HEAD/src/utilities/policies/__tests__/relayStylePagination.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
👩‍🔬 needs-more-tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zanechua @alessbell @jerelmiller @phryneas